Skip to content

Conversation

@picnixz
Copy link
Member

@picnixz picnixz commented Jun 21, 2025

@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @picnixz for commit ae37e95 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F135789%2Fmerge

If you want to schedule another build, you need to add the 🔨 test-with-refleak-buildbots label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-refleak-buildbots Test PR w/ refleak buildbots; report in status section label Jun 21, 2025
@picnixz picnixz force-pushed the fix/exception/sys-exit-133548 branch from bcc2970 to 6b65314 Compare June 23, 2025 07:52
}
else {
PyErr_SetObject(PyExc_SystemExit, status);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this a problem with how PyErr_SetObject creates the exception? Shouldn't it be fixed there?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's a bit unfortunate because that's how _PyErr_CreateException handles tuples:

static PyObject*
_PyErr_CreateException(PyObject *exception_type, PyObject *value)
{
    PyObject *exc;

    if (value == NULL || value == Py_None) {
        exc = _PyObject_CallNoArgs(exception_type);
    }
    else if (PyTuple_Check(value)) {
        exc = PyObject_Call(exception_type, value, NULL);
    }
    else {
        exc = PyObject_CallOneArg(exception_type, value);
    }

    if (exc != NULL && !PyExceptionInstance_Check(exc)) {
        PyErr_Format(PyExc_TypeError,
                     "calling %R should have returned an instance of "
                     "BaseException, not %s",
                     exception_type, Py_TYPE(exc)->tp_name);
        Py_CLEAR(exc);
    }

    return exc;
}

Unless you want me to change _PyErr_CreateException to prevent the fast paths, it could be annoying for downstream users.

if (PyTuple_Check(status)) {
/* Make sure that tuples are not flattened during normalization. */
/* Make sure that tuples are not flattened during normalization
* due to the fast path for tuples in _PyErr_CreateException(). */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not a "fast path" if removing it changes behaviour. We need to understand whether _PyErr_CreateException has a bug or not.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it was designed as such for conveniency. It was added in 3a84097 but AFAICT, that was just refactoring of c0dc92a (1997) which already had this fast path.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants